Skip to content

Encapsulate visualization#196

Open
simonroeschhso wants to merge 4 commits intoskogsbaer:masterfrom
simonroeschhso:encapsulate-visualization
Open

Encapsulate visualization#196
simonroeschhso wants to merge 4 commits intoskogsbaer:masterfrom
simonroeschhso:encapsulate-visualization

Conversation

@simonroeschhso
Copy link

No description provided.

@skogsbaer skogsbaer self-requested a review March 20, 2026 10:25
Copy link
Owner

@skogsbaer skogsbaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work, it's much better now. I asked Claude Code for more improvements and there are indeed some thing we should make better. Below are Claude's suggestions, I copied them verbatim.

Some comments by myself (Stefan)

  • the intent of most files become much clearer, if there is a short comment at the top of the file explaining the purpose of the file
  • the README is good, but it's a bit to low-level (e.g. mentioning exact line numbers which are obsolete anyway after the next edit). You should describe the architecture on a higher-level. Maybe showing a simple mermaid diagram. Claude Code can help there too.

Frontend Refactoring Suggestions by Claude Code

1. Eliminate the navigation round-trip

Current behaviour: Clicking a navigation button (prev/next/first/last) or moving the slider round-trips through the extension host:

webview click → CustomEvent → adapter → vscode.postMessage({onClick})
  → visualization_panel.ts: updateTraceIndex() + postReset()
    → postMessage({reset, trace: [...all elements...], index})
      → adapter → CustomEvent → webview: traceIndex = msg.index; renderCurrent()

The webview already holds the full trace array and already does local navigation in the if (!isVscode) block for standalone-browser mode. VS Code mode should work the same way.

Fix: Remove the if (!isVscode) guard. Navigation always runs locally in the webview. After updating traceIndex and rendering, send a single lightweight message back for line highlighting only:

// webview.ts
function navigate(type: "first" | "prev" | "next" | "last") {
    const max = Math.max(0, trace.length - 1);
    switch (type) {
        case "first": traceIndex = 0; break;
        case "prev":  traceIndex = Math.max(0, traceIndex - 1); break;
        case "next":  traceIndex = Math.min(max, traceIndex + 1); break;
        case "last":  traceIndex = max; break;
    }
    renderCurrent();
    if (isVscode && trace.length > 0) {
        vscode.postMessage({ command: "highlight",
            filePath: trace[traceIndex].filePath,
            line: trace[traceIndex].line });
    }
}

visualization_panel.ts drops _traceIndex, onClick, onSlide, and updateTraceIndex. It only needs to handle the new "highlight" message and call updateLineHighlight().

postReset() is then only called on: initial load, panel refocus (onDidChangeViewState), and after the trace is fully cached.


2. Stop sending the full trace array on every navigation step

postReset() serializes and transmits the entire accumulated trace array every time the user navigates. For programs with many trace steps this is significant unnecessary IPC overhead.

This is fixed as a direct consequence of suggestion 1 — once navigation is local, postReset() is no longer called on each step. The "reset" message is only ever sent with the full trace on initial load or after trace completion, which is appropriate.


3. Remove dead message types

"updateButtons" and "updateContent" are handled in both vscode-host-adapter.ts and webview.ts but are never posted from visualization_panel.ts. They are dead code.

Fix: Delete the corresponding case blocks in vscode-host-adapter.ts and the two window.addEventListener("programflow:updateButtons", ...) / window.addEventListener("programflow:updateContent", ...) handlers in webview.ts.


4. Simplify the adapter's outgoing direction

After suggestion 1, navigation events no longer need to leave the webview. The programflow:onClick and programflow:onSlide outgoing listeners in vscode-host-adapter.ts become dead code and can be removed.

The adapter's responsibilities shrink to a clear, minimal contract:

  • Incoming (extension → webview): translate window.message events for "reset" and "append" into CustomEvents
  • Outgoing (webview → extension): forward "select" and "highlight" via vscode.postMessage

The standalone-browser use case is fully preserved: acquireVsCodeApi is still mocked, the adapter still loads first, and webview code remains decoupled from the VS Code API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants